virtual-scroll: add e2e tests for autosize scroll strategy #11345
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
crisbeto
approved these changes
May 15, 2018
| /** Immediately scrolls the viewport to the given offset. */ | ||
| function scrollViewportTo(viewportEl: any, offset: number, done: () => void) { | ||
| viewportEl.scrollTop = offset; | ||
| window.requestAnimationFrame(() => done()); |
There was a problem hiding this comment.
I'm not sure whether this wouldn't introduce flakes in the CI. Most browsers will pause or throttle the requestAnimationFrame calls if the browser somehow ends up being in the background.
There was a problem hiding this comment.
Interesting, I need to pass some time since the scroll events are sampled every animation frame. If this turns out to be flaky I can switch to browser.sleep(16). Let's just see what happens with this first though
| <section class="demo-virtual-scroll-variable-size"> | ||
| <h3>Random size</h3> | ||
| <cdk-virtual-scroll-viewport class="demo-viewport" autosize | ||
| style="height:300px; width:300px; box-shadow:0 0 0 1px black"> |
There was a problem hiding this comment.
Consider putting these styles in a stylesheet?
tools/gulp/tasks/e2e.ts
Outdated
| @@ -31,6 +32,27 @@ task('e2e', sequenceTask( | |||
| 'screenshots', | |||
| )); | |||
|
|
|||
| task('e2e:watch', sequenceTask( | |||
There was a problem hiding this comment.
These tasks should have docstrings on them.
| @@ -24,6 +28,14 @@ const rollupMatEntryPoints = matSecondaryEntryPoints.reduce((globals: any, entry | |||
| return globals; | |||
| }, {}); | |||
|
|
|||
| /** Object with all cdk-experimental entry points in the format of Rollup globals. */ | |||
| const rollupCdkExperimentalEntryPoints = | |||
| cdkExperimentalSecondaryEntryPoints.reduce((globals: any, entryPoint: string) => { | |||
There was a problem hiding this comment.
Looks like this exact reduce block is being repeated three times now. While we're at it, might be a good idea to move it out into a function.
|
|
||
|
|
||
| /** Checks if the given element is visible in the given viewport. */ | ||
| async function isVisibleInViewport(el: ElementFinder, viewport: ElementFinder): Promise<boolean> { |
There was a problem hiding this comment.
nit: consider naming this something along the lines of isCompletelyInViewport? isVisibileInViewport could apply to something that's partially in the viewport.
There was a problem hiding this comment.
It does apply to things that are partially in the viewport, that is intentional.
mmalerba
added a commit
that referenced
this pull request
May 22, 2018
* feat(virtual-scroll): fixed size virtual scroll (#9316) * feat(virtual-scroll): fixed size virtual scroll * address some comments * change VirtualScrollStrategy interface a bit * address some more comments * fix lint & build * chore: move virtual-scroll to cdk-experimental (#9974) * virtual-scroll: simplify scroll listener logic (#10102) * virtual-scroll: only move views that need to be moved (#10099) * virtual-scroll: only move views that need to be moved * address comments * virtual-scroll: switch `throttleTime` to `sampleTime` (#10179) * virtual-scroll: switch throttleTime to sampleTime * add comment * virtual-scroll: allow user to pass `Observable<T[]>` (#10158) * virtual-scroll: rename `Range` to `ListRange` to avoid confusion with native `Range` (#10220) * virtual-scroll: add autosize scroll strategy (#10219) * rename fixed size virtual scroll directive * add autosize virtual scroll strategy * add item size estimator class * add logic for jumping rendered content based on scroll position * address comments * virtual-scroll: add `onContentRendered` hook to `VirtualScrollStrategy` (#10290) * virtual-scroll: add `onContentRendered` hook to `VirtualScrollStrategy` * address comemnts * virtual-scroll: add incremental scroll logic in `AutosizeVirtualScrollStrategy` (#10504) * virtual-scroll: add incremental scroll logic in `AutosizeVirtualScrollStrategy`. This still has a couple issues that need to be ironed out and it doesn't have the code for correcting the error between the predicted and actual scroll position. (See various TODOs for additional things that need work). * fix lint * address comments * address comments * fix bazel * fix devapp * fix lint * cleanup * virtual-scroll: rewrite offset in terms of "to-top" and fix a bug where items were removed too soon (#10986) * rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * address comments * virtual-scroll: address amcdnl's feedback (#10988) * rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * virtual-scroll: address amcdnl's feedback * virtual-scroll: fix updating when data changes and add trackBy demos (#11085) * virtual-scroll: add logic to correct the scroll position as user move… (#11137) * virtual-scroll: add logic to correct the scroll position as user moves toward the top * address comments * fix(scrolling): adds right to fix pushed content (#11192) * fix(scrolling): adds right to fix pushed content * chore: address feedback * chore: address pr feedback * virtual-scroll: add tests for fixed size items (#11255) * virtual-scroll: add tests for fixed size items * address comments * fix bug in fixed size strategy * fix bazel build * virtual-scroll: add tests for `cdkVirtualFor` logic (#11275) * merge fixed size test components into one * add tests for cdkVirtualFor logic * allow undefined to be explicitly passed as trackBy * fix bazel build * address comments * add some basic tests (#11295) * fix lint * virtual-scroll: add e2e tests for autosize scroll strategy (#11345) * set up virtual scroll page in e2e app * add gulp task for e2e:watch * add e2e tests for autosize * address comments * address comments * fix lint and tests * fix bad rebase * fix(scrolling): scrollbar jump when data change on scroll (#11193) * fix(scrolling): data change not updating size * chore: remove todo * chore: more performant fix for jump
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #10180